Give integer literals a sign instead of relying on negation expressions #136860

oli-obk · 2025-02-11T10:57:33Z

A lot of situations in the compiler check for unary negation of literals, which I want to replace entirely at some point. I think the only place that should have to handle such things is the overflowing literals lint to detect double negation. Everywhere else we should just have the concept of negative literals

See also the corresponding MCP: rust-lang/compiler-team#835

oli-obk · 2025-02-11T11:00:31Z

@bors try @rust-timer queue

bors · 2025-02-11T11:01:46Z

⌛ Trying commit 25e4dd9 with merge 994d560...

Give integer literals a sign instead of relying on negation expressions r? `@ghost` A lot of situations in the compiler check for unary negation of literals, which I want to replace entirely at some point. I think the only place that should have to handle such things is the overflowing literals lint to detect double negation. Everywhere else we should just have the concept of negative literals.

compiler/rustc_ast/src/ast.rs

tests/ui/enum/enum-discrim-too-small.stderr

bors · 2025-02-11T12:54:47Z

☀️ Try build successful - checks-actions
Build commit: 994d560 (994d560d7ca8e0cae1a0ed123e070dccfdf344a1)

rust-timer · 2025-02-11T15:42:32Z

Finished benchmarking commit (994d560): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.3%]	12
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.4%, -0.3%]	17
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -0.7%, secondary -2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-2.9%	[-6.7%, -2.0%]	12
All ❌✅ (primary)	-0.7%	[-3.1%, 1.7%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 785.339s -> 786.428s (0.14%)
Artifact size: 348.32 MiB -> 348.25 MiB (-0.02%)

tests/ui/lint/lint-type-overflow2.stderr

oli-obk · 2025-02-26T12:02:09Z

src/tools/clippy/tests/ui/manual_div_ceil_with_feature.fixed

-    let _ = y.div_ceil(-7);
-    let _ = y.div_ceil(-7);
-    let _ = y.div_ceil(-7);
+    let _ = (y + -8) / -7;
+    let _ = (-8 + y) / -7;
+    let _ = (y - 8) / -7;


This is actually a bugfix.

rustbot · 2025-02-26T14:46:43Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @rust-lang/wg-const-eval

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

HIR ty lowering was modified

cc @fmease

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann

Some changes occurred in match checking

cc @Nadrieril

oli-obk · 2025-02-26T14:49:28Z

The 2 weeks of the MCP at rust-lang/compiler-team#835 is formally over

bors · 2025-03-02T02:26:00Z

☔ The latest upstream changes (presumably #137752) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2025-03-12T15:44:23Z

After reading the diff I think this a net negative even in case of HIR.

I understand that it is a follow up to the work on patterns in #134248 and its predecessors.
In case of patterns, this indeed makes sense, because the set of expressions in patterns is limited to some forms including negated literal expressions (*).

(*) Not even literals, because -$expr is also supported, despite not being a literal, there are effectively parentheses there due to the macro variable.

I don't think the case of patterns should be extended to the rest of the language.
If some lints need to treat negated and non-negated literal expressions similarly, then maybe some helper methods will help, perhaps with flavors to differentiate between -10 and less literal-looking forms like -(10), -$lit or even - /* space */ 10.

Some type error treatment also becomes less natural with this approach, previously -"string" could be reported as an error just because strings do not implement Neg in general, not it requires a special case for literals.

As for the early compilation, I think the right mindset there is that negative literals simply don't exist, and if something looks like a negative literal to you, then it's just imagination. So I clearly disagree with the idea of "just having the concept of negative literals everywhere".
That applies to token streams, lexer, parser, and AST as well because AST is intertwined with the previous things pretty closely.
In this PR the rustc_ast* part of the diff looks mostly like an introduction of a great confusion into that model.

oli-obk added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. needs-mcp This change is large enough that it needs a major change proposal before starting work. labels Feb 11, 2025

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 11, 2025

This comment has been minimized.

Sign in to view

oli-obk mentioned this pull request Feb 11, 2025

Give integer literals a sign instead of relying on negation expressions rust-lang/compiler-team#835

Closed

3 tasks

Nadrieril reviewed Feb 11, 2025

View reviewed changes

compiler/rustc_ast/src/ast.rs Outdated Show resolved Hide resolved

Nadrieril reviewed Feb 11, 2025

View reviewed changes

tests/ui/enum/enum-discrim-too-small.stderr Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 11, 2025

traviscross reviewed Feb 11, 2025

View reviewed changes

tests/ui/lint/lint-type-overflow2.stderr Outdated Show resolved Hide resolved

oli-obk force-pushed the signed-literals branch from 25e4dd9 to 515be05 Compare February 12, 2025 10:08

This comment has been minimized.

Sign in to view

oli-obk force-pushed the signed-literals branch from 515be05 to 4d3981a Compare February 12, 2025 11:38

petrochenkov self-assigned this Feb 12, 2025

This comment was marked as resolved.

Sign in to view

oli-obk force-pushed the signed-literals branch from 4d3981a to 09304f1 Compare February 25, 2025 09:01

This comment has been minimized.

Sign in to view

oli-obk force-pushed the signed-literals branch from ab82ba5 to dc60c28 Compare February 26, 2025 10:36

This comment has been minimized.

Sign in to view

oli-obk force-pushed the signed-literals branch from dc60c28 to 3cc8a08 Compare February 26, 2025 11:57

oli-obk mentioned this pull request Feb 26, 2025

Give integer and float literals a sign rust-lang/compiler-team#843

Closed

3 tasks

oli-obk added 3 commits February 26, 2025 12:21

Carry sign through literals

25a09c4

Avoid loading a snippet and render the literal directly

66fc247

Fix fallout in tools

63e3924

This comment has been minimized.

Sign in to view

Use an enum for the sign instead of a bool

e473e47

oli-obk force-pushed the signed-literals branch from 3cc8a08 to e473e47 Compare February 26, 2025 14:38

oli-obk commented Feb 26, 2025

View reviewed changes

oli-obk marked this pull request as ready for review February 26, 2025 14:46

apiraino removed the needs-mcp This change is large enough that it needs a major change proposal before starting work. label Mar 3, 2025

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 12, 2025

oli-obk closed this Mar 13, 2025

oli-obk mentioned this pull request Mar 18, 2025

RFC: const ergonomics for NonZero<T> rust-lang/rfcs#3786

Open

Give integer literals a sign instead of relying on negation expressions #136860

Give integer literals a sign instead of relying on negation expressions #136860

Uh oh!

Conversation

oli-obk commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Feb 11, 2025

Uh oh!

This comment has been minimized.

bors commented Feb 11, 2025

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

bors commented Feb 11, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Feb 11, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

rustbot commented Feb 26, 2025

Uh oh!

oli-obk commented Feb 26, 2025

Uh oh!

bors commented Mar 2, 2025

Uh oh!

petrochenkov commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

oli-obk commented Feb 11, 2025 •

edited

Loading